Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: introduce createFactory() and createHandlers() #1743

Merged
merged 5 commits into from
Nov 29, 2023
Merged

Conversation

yusukebe
Copy link
Member

@yusukebe yusukebe commented Nov 28, 2023

I made #1722 a draft and create this new PR instead it.

This PR introduce Factory class, createFactory(), and createHandlers() in Factory exported from hono/factory.

import { createFactory } from 'hono/factory'
import { logger } from 'hono/logger'

const factory = createFactory<Env>()

const middleware = factory.createMiddleware(async (c, next) => {
  c.set('foo', 'bar')
  await next()
})

const handlers = factory.createHandlers(logger(), middleware, (c) => {
  return c.json(c.var.foo)
})

app.get('/api', ...handlers)

This is useful for defining handlers outside of app.get('/api', ...). Using this approach, you can structure your application in a Ruby on Rails-like Controller pattern, as discussed in #1072.

A key feature is its ability to correctly infer types.

Screenshot 2023-11-29 at 5 56 32

Why we can't call createHandlers()directly

Ideally, you might write shortly as follows:

import { createHandlers } from 'helper/factory'

However, there is a problem that passing Env as follows will break other types:

createHandlers<Env>()

createMiddleware()

You can use createMiddleware as before:

import { createMiddleware } from 'helper/factory'

Author should do the followings, if applicable

  • Add tests
  • Run tests
  • yarn denoify to generate files for Deno

@yusukebe
Copy link
Member Author

Hi @usualoma

Please review this one!

Copy link
Member

@usualoma usualoma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's good 👍
It looked like a pain to have to createFactory() when I first saw it, but now I understand that it's a necessary process.

@yusukebe
Copy link
Member Author

@usualoma

Thanks! I'll merge this into the "next" later.

@yusukebe yusukebe merged commit 6dd1a77 into next Nov 29, 2023
11 checks passed
@yusukebe yusukebe deleted the feat/factory-class branch November 29, 2023 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants